Skip to content

[DX-2353] docs: clarify max_record_size and omit_detailed_recording do not apply to aggregate pumps - #2800

Open
sharadregoti wants to merge 8 commits into
mainfrom
DX-2353-aggregate-pump-doc-caveat
Open

[DX-2353] docs: clarify max_record_size and omit_detailed_recording do not apply to aggregate pumps#2800
sharadregoti wants to merge 8 commits into
mainfrom
DX-2353-aggregate-pump-doc-caveat

Conversation

@sharadregoti

@sharadregoti sharadregoti commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

User description

Jira

DX-2353 - max_record_size and omit_detailed_recording are documented identically for every pump type, including aggregate pumps, but have no effect there.

Change

Adds a caveat to the pumps.mongoaggregate.omit_detailed_recording, pumps.mongoaggregate.max_record_size, pumps.sqlaggregate.omit_detailed_recording, and pumps.sqlaggregate.max_record_size entries on the Tyk Pump environment-variable reference, clarifying they have no effect on aggregate pumps because aggregated analytics records never include raw_request or raw_response data. A customer had followed the docs and set max_record_size on an aggregate pump expecting it to control aggregate record size.

Why this edits an auto-generated snippet

snippets/pump-config.mdx is normally synced from the Tyk Pump source. The durable fix is in tyk-pump#1090, which adds the same caveat to the originating Go struct doc comments in config.go. This PR applies the identical wording directly so the correction ships immediately, ahead of the next config-docs sync. Once tyk-pump#1090 merges, the next sync regenerates the same text, so there is no drift.


PR Type

Documentation


Description

  • Clarify aggregate pump setting limitations

  • Note omit_detailed_recording has no effect

  • Note max_record_size has no effect

  • Explain aggregate records lack raw payloads


Diagram Walkthrough

flowchart LR
  a["Aggregate pump settings"]
  b["omit_detailed_recording caveat"]
  c["max_record_size caveat"]
  d["Clarified aggregate pump behavior"]
  a -- "documents" --> b
  a -- "documents" --> c
  b -- "explains" --> d
  c -- "explains" --> d
Loading

File Walkthrough

Relevant files
Documentation
pump-config.mdx
Document no-op settings for aggregate pumps                           

snippets/pump-config.mdx

  • Added caveat under pumps.mongoaggregate.omit_detailed_recording
  • Added caveat under pumps.mongoaggregate.max_record_size
  • Added matching caveats for pumps.sqlaggregate settings
  • Explained aggregate records exclude raw_request and raw_response
+12/-0   

@probelabs

probelabs Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

This pull request is a documentation update that clarifies the behavior of two configuration settings, max_record_size and omit_detailed_recording, for Tyk's aggregate pumps (mongoaggregate and sqlaggregate). It adds an explicit note explaining that these settings have no effect on aggregate pumps because their data does not include the raw_request or raw_response payloads that these settings are designed to manage.

Files Changed Analysis

  • File Modified: snippets/pump-config.mdx
  • Changes: 12 lines added, 0 deleted.
  • Summary: The change consists of adding the same explanatory note to four sections within the Tyk Pump configuration documentation. The note is applied to both the omit_detailed_recording and max_record_size settings for the mongoaggregate and sqlaggregate pump types.

Architecture & Impact Assessment

  • What this PR accomplishes: It corrects the documentation to prevent users from misconfiguring aggregate pumps under the false assumption that these settings will control record size. This aligns the documentation with the actual system behavior, reducing potential support issues and user confusion.
  • Key technical changes introduced: This is a documentation-only change. No application code or system behavior is altered.
  • Affected system components: The only component affected is the Tyk Pump configuration documentation. There is no impact on the runtime of Tyk Pump or any other part of the Tyk stack.
flowchart TD
    subgraph Tyk Pump Documentation
        A["pump-config.mdx"] -- Documents --> B & C
    end

    subgraph Configuration Settings
        B["pumps.mongoaggregate.*"]
        C["pumps.sqlaggregate.*"]
    end

    subgraph Clarification Added
        D["Note: omit_detailed_recording has no effect"]
        E["Note: max_record_size has no effect"]
    end

    B --> D & E
    C --> D & E
Loading

Scope Discovery & Context Expansion

  • The scope of this PR is narrowly focused on a single documentation file.
  • The PR description provides crucial context: snippets/pump-config.mdx is an auto-generated file. This manual edit is a short-term fix to get the correction published quickly.
  • The long-term, durable fix is being implemented in the source repository in pull request tyk-pump#1090. This ensures that when the documentation is next regenerated, the clarification will be preserved, preventing any configuration drift.
Metadata
  • Review Effort: 1 / 5
  • Primary Label: documentation

Powered by Visor from Probelabs

Last updated: 2026-08-31T11:59:58.410Z | Triggered by: pr_updated | Commit: d1fd4a5

💡 TIP: You can chat with Visor using /visor ask <your question>

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌

1090 - Not compliant

Non-compliant requirements:

  • Add a new Portal and Dashboard architecture section to the portal introduction page.
  • Explain that the Developer Portal operates as a client of the Tyk Dashboard API and does not connect directly to the Gateway.
  • Include a Mermaid diagram showing the data flow between Portal, Dashboard, Gateway, and upstream services.
  • Add a cross-reference note in the setup guide linking to the new architecture section.

Requires further human verification:

  • Verify that the Mermaid diagram renders correctly.
  • Verify that the cross-reference links work.
  • Verify that the architecture explanation is accurate.
⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@probelabs

probelabs Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

✅ Technical-accuracy Check Passed

No technical-accuracy issues found – changes LGTM.

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Technical-accuracy Check Passed

No technical-accuracy issues found – changes LGTM.

\n\n

✅ Security Check Passed

No security issues found – changes LGTM.

\n\n

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

✅ Quality Check Passed

No quality issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2026-08-31T11:59:40.625Z | Triggered by: pr_updated | Commit: d1fd4a5

💡 TIP: You can chat with Visor using /visor ask <your question>

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@github-actions

Copy link
Copy Markdown

STE100 style check (advisory)

1 errors, 6 warnings across 1 file(s).

File Errors Warnings
snippets/pump-config.mdx 1 6
Full findings
warning[passive-voice]: possible passive voice "are pulled"
 --> snippets/pump-config.mdx:6:28
  |
6 | analytics records in Redis are pulled. If it's set, `storage_expiration_time` is used to
  |                            ^^^^^^^^^^
  = help: rewrite in the active voice: name the actor before the verb
warning[passive-voice]: possible passive voice "is used"
 --> snippets/pump-config.mdx:6:79
  |
6 | analytics records in Redis are pulled. If it's set, `storage_expiration_time` is used to
  |                                                                               ^^^^^^^
  = help: rewrite in the active voice: name the actor before the verb
warning[passive-voice]: possible passive voice "is enabled"
 --> snippets/pump-config.mdx:13:85
   |
13 | The number of seconds for the analytics records TTL. It only works if `purge_chunk` is
   |                                                                                     ^^
   = help: rewrite in the active voice: name the actor before the verb
warning[ing-form]: "connection_string" uses the -ing form
 --> snippets/pump-config.mdx:37:5
   |
37 |     "connection_string": "host=sql_host port=sql_port user=sql_usr dbname=dbname password=sql_pw",
   |     ^^^^^^^^^^^^^^^^^
   = help: use the infinitive or an approved form
warning[ing-form]: "table_sharding" uses the -ing form
 --> snippets/pump-config.mdx:38:5
   |
38 |     "table_sharding": false
   |     ^^^^^^^^^^^^^^
   = help: use the infinitive or an approved form
error[sentence-length]: descriptive sentence has 28 words; the limit is 25
 --> snippets/pump-config.mdx:638:1
    |
638 | "dogstatsd": {
    | ^^^^^^^^^^^^^^
    = help: split the sentence into shorter sentences
warning[ing-form]: "initializing" uses the -ing form
 --> snippets/pump-config.mdx:665:36
    |
665 | [May 10 15:23:44]  INFO dogstatsd: initializing pump
    |                                    ^^^^^^^^^^^^
    = help: use the infinitive or an approved form

Advisory only, this does not block merging. Run python3 scripts/ste_check.py locally to check before pushing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant